Skip to content

fix: #27 stop the deploy job failing and silently skipping the API restart #39

Jump to bottom
Merged
merged 2 commits into from
Oct 1, 2026
Merged

fix: #27 stop the deploy job failing and silently skipping the API restart #39

merged 2 commits into from
Oct 1, 2026
+51 −11

Conversation

erikhfj
Copy link
Member

@erikhfj erikhfj commented Oct 1, 2026

Why

Rachel: "deployen feilet etter forrige merge når CI skulle restarte GraphQL API-et. Ser ut som det også er noe rart med sjekken av crate-api.service ... for to dager siden ble den skippet selv om servicen allerede fantes."

Both symptoms are real and they have different causes.

Bug 1: the check skipped a service that was there

if systemctl list-unit-files | grep -q '^crate-api.service'; then

grep -q exits at the first match and closes the pipe. systemctl then gets SIGPIPE, prints Failed to print table: Broken pipe and exits 1. With pipefail the whole pipeline returns 1, so the if takes the else branch and reports "crate-api.service not installed yet, skipping" while the unit is sitting right there in /etc/systemd/system/.

Whether it matched was a race on how much systemctl had flushed before grep closed the pipe. That is the intermittency.

I reproduced it in isolation:

result
small output, match near the end if took THEN
200 000 lines, match on line 1 if took ELSE

Replaced with [ -f /etc/systemd/system/crate-api.service ]. No pipe, no SIGPIPE, no pipefail interaction.

Bug 2: sudo cannot authenticate

From run #11 on the merge of #36:

sudo: A terminal is required to authenticate
##[error]Process completed with exit code 1.

The self-hosted runner has no TTY. This needs a NOPASSWD rule on the VM, which is not a change I can make in this file. One command, called out in the PR comment.

Frontend publishing happens before this step and was always fine, which is why the site looked deployed while the job was red.

Bug 3, latent: it would have restarted stale code

crate-api.service on the VM has WorkingDirectory=/home/erikhfj/T19-Project-2/server and ExecStart=…/node dist/index.js. That is the prototype I rsynced to the VM on 23 Sep, still sitting there. server/ does not exist on main until M2. So a fixed sudo would have restarted the wrong process.

The step now also gates on server/ being in the repository:

  • crate-api.service absent, or server/ absent -> skip, with the reason printed
  • server/ present but server/dist/index.js missing -> fail loudly, that is a real build problem
  • otherwise restart and curl the endpoint

What was tested

The four decision paths were exercised directly:

case 1: no unit file, no server/          -> skip
case 2: unit present, no server/          -> skip        (the M1 reality)
case 3: unit present, server/ not built   -> error
case 4: unit present, server built        -> would restart
case 5: old pipefail + grep -q, small     -> THEN
case 6: old pipefail + grep -q, 200k rows -> ELSE         (the bug)

YAML parses, prettier clean, and the job structure is unchanged apart from the one step.

What this does not fix

Deploying the server is M2 work. Right now the deploy job only publishes the frontend and deliberately does not touch crate-api. When server/ lands, this step will start restarting it, and M2 needs to add the step that copies the server build to the path the unit expects.

Closes

Addresses the deploy half of #27. The workflow is already on main from #37.

@erikhfj
fix: #27 stop the deploy job failing and silently skipping the API re…
436fa25 
…start

Two bugs, both reported by rachelks after #36 merged and the deploy went
red.

1. `systemctl list-unit-files | grep -q '^crate-api.service'` took the
   else branch even when the unit was installed. `grep -q` exits at the
   first match and closes the pipe, `systemctl` gets SIGPIPE and dies,
   and with `pipefail` the pipeline reports failure so the `if` falls
   through to "not installed yet, skipping". Whether it matched or not
   depended on how much output `systemctl` had flushed before grep
   closed, which is why it was intermittent. The log said:

       Failed to print table: Broken pipe
       crate-api.service not installed yet (arrives in M2). Skipping.

   Replaced with `[ -f /etc/systemd/system/crate-api.service ]`. No
   pipe, no SIGPIPE, no pipefail interaction.

2. `sudo systemctl restart crate-api` fails because the self-hosted
   runner has no TTY and no passwordless rule:

       sudo: A terminal is required to authenticate

   Needs a NOPASSWD rule scoped to exactly these commands. That is a
   change on the VM, not in this file, so it is called out in the step
   comment and in the PR.

Also gated the restart on `server/` actually being in the repository.
The unit on the VM still points at prototype code from 23 Sep, and
`server/` does not exist on main until M2. Restarting unconditionally
would bring that stale process back. Now it skips with a stated reason
while M1 is the current state, and fails loudly if `server/` exists but
was never built.

Reported by rachelks.

Co-Authored-By: opencode <noreply@opencode.ai>
@erikhfj erikhfj requested a review from a team as a code owner October 1, 2026 19:28
@erikhfj
fix: #27 poll for the API instead of sleeping two seconds
1f6172c 
The health check after `systemctl restart crate-api` returned 503
through the Apache proxy on a real run even though the service was
active. `Type=simple` makes systemd report active as soon as the process
is spawned, well before Node has bound the port.

Measured three restarts on the VM: the API answers again after 1878,
1850 and 1864 ms. A fixed `sleep 2` therefore leaves about 120 ms of
margin, which is not enough to be reliable.

Replaced with a poll: up to 30 attempts, one second apart, each with a
two second curl timeout. Reports 200 as soon as it is up, and fails with
the last status code if it never is.

Verified by running the step against the live unit on the VM.

Co-Authored-By: opencode <noreply@opencode.ai>
@erikhfj
Copy link
Member Author

erikhfj commented Oct 1, 2026

@rachelks both symptoms you reported were real and they had different causes. Details above, but the short version:

The skip that made no sense. My check was systemctl list-unit-files | grep -q '^crate-api.service'. grep -q exits at the first match and closes the pipe. systemctl then gets SIGPIPE, dies, and with pipefail the pipeline reports failure, so the if fell through to "not installed yet, skipping" while the unit was right there. The log said Failed to print table: Broken pipe. Whether it matched depended on how much systemctl had flushed before grep closed, which is why it was intermittent. Now it is [ -f /etc/systemd/system/crate-api.service ], no pipe at all.

The deploy going red. sudo: A terminal is required to authenticate. The runner has no TTY. That is fixed with a NOPASSWD rule on the VM scoped to exactly three systemctl commands, installed and verified.

A third one I found while fixing it. The unit still points at prototype code from 23 Sep, and server/ is not on main until M2. A fixed sudo would have restarted the wrong process. The step now skips unless server/ is in the repository, and fails loudly if server/ exists but was never built.

And a fourth. After a restart, Type=simple reports active before Node binds the port. Measured three restarts on the VM: 1878, 1850, 1864 ms to answer. sleep 2 left 120 ms of margin and /graphql returned 503 through Apache at least once. Now it polls until it gets 200.

Thanks for the report. The "skipped even though it exists" observation was the part that made the SIGPIPE bug findable, otherwise it would have looked like systemd being flaky.

rachelks
rachelks approved these changes Oct 1, 2026
View reviewed changes
Copy link
Member

@rachelks rachelks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for fixing this and explaining what was causing the CI issues :)

@rachelks rachelks merged commit 4c80fd1 into main Oct 1, 2026
2 checks passed
@rachelks rachelks deleted the fix/27-ci-deploy branch October 15, 2026 08:30
Sign in to join this conversation on GitHub.
Reviewers

@rachelks rachelks rachelks approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@erikhfj @rachelks